Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix navigation sidebar being too wide for some phones - issue #421 #590

Merged
merged 10 commits into from
Dec 11, 2018
Merged

fix navigation sidebar being too wide for some phones - issue #421 #590

merged 10 commits into from
Dec 11, 2018

Conversation

JStein92
Copy link
Contributor

@JStein92 JStein92 commented Dec 3, 2018

#421

This PR is a:

[ ] New feature
[ ] Enhancement/Optimization
[ ] Refactor
[ x ] Bugfix
[ ] Test for existing code
[ ] Documentation

Summary

When this pull request is merged, it will fix a few bugs-

  1. Navigation sidebar was too wide for some phones. (set to 360px)
  2. User was not able to scroll down on navigation sidebar when it's height was greater than the window height
  3. After signing in or creating an account, the "back arrow" methods and "title" were not acting correctly

Additional information

I also added a bottom margin to the create account buttons so they would not be flush against the nav sidebar bottom

@JStein92
fix navigation sidebar being too wide for some phones, fix bug which …
af678f0 
…caused back method/titles on nav to be wrong after creating account and signing in
@vercel
Copy link

vercel bot commented Dec 3, 2018

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

@coveralls
Copy link

coveralls commented Dec 3, 2018 •
edited
Loading

Coverage Status

Coverage decreased (-0.04%) to 21.52% when pulling 1ef41f4 on bargreenellingson:scroll-fix into 661d724 on magento-research:release/2.0.

@supernova-at
Copy link
Contributor

Code looks good!

Haven't gotten a chance to verify this one yet.

@PWAStudioBot
Copy link
Contributor

Fails
🚫

The following file(s) were not formatted with prettier. Make sure to execute npm run prettier locally prior to committing.

packages/venia-concept/src/components/Navigation/navigation.js

Generated by 🚫 dangerJS

@zetlen zetlen self-requested a review December 11, 2018 19:20
zetlen
zetlen approved these changes Dec 11, 2018
View reviewed changes
Copy link
Contributor

@zetlen zetlen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great to me, though I observed while testing that we have a UX issue with our input fields in iOS Safari: the browser zooms in automatically on focused input fields. It looks like this is because Safari requires text inputs to be minimum 16px font size, but we can address that in a separate PR.

@zetlen zetlen merged commit 7d4bae2 into magento:release/2.0 Dec 11, 2018
@zetlen zetlen mentioned this pull request Dec 11, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zetlen zetlen zetlen approved these changes

@supernova-at supernova-at supernova-at approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@JStein92 @coveralls @supernova-at @PWAStudioBot @zetlen